Adding line number printing when output is piped out #2983
Conversation
tests/integration_tests.rs
Outdated
| @@ -1779,7 +1779,7 @@ fn file_with_invalid_utf8_filename() { | |||
| .arg(file_path.as_os_str()) | |||
| .assert() | |||
| .success() | |||
| .stdout("dummy content\n"); | |||
| .stdout(" 1 dummy content\n"); | |||
There was a problem hiding this comment.
I'm curious, is this really the expected behavior? The test invokes bat without any line number style component arguments. I think it should stay as it was before - according to the issue being solved, line numbers should only be displayed when piping when the command line argument was explicitly passed, no?
There was a problem hiding this comment.
good catch!
I added a check for handle type here
There was a problem hiding this comment.
this doesn't pass the test now, it seems like there may be an issue with the filename and the is_terminal() function or something like that
fn is_terminal(&self) -> bool
───────────────────────────────────────────
Returnstrueif the descriptor/handle refers to a terminal/tty.On platforms where Rust does not know how to detect a terminal yet, this will return
false. This will also returnfalseif an unexpected error occurred, such as from
passing an invalid file descriptor.
There was a problem hiding this comment.
debugging it shows self.config.style_components.numbers() returns true, is there a different way to know if line numbers are requested that i'm missing?
There was a problem hiding this comment.
My guess would be the following:
The SimplePrinter is used whenever stdout is not interactive, --color is not set to always, --decorations is not set to always and --force-colorization is not set either (see app.rs, config.rs and controller.rs). --style controls which style components (such as header or line numbers) are used. The default for --style is changes,grid,header-filename,numbers,snip. This default currently stays the same even if loop_through == true (which means that SimplePrinter will be used). Currently, SimplePrinter just does not care about --style.
So, when loop_through == true, the default for --style should be plain instead. Then your changes should work since they do when you specify --style=plain explicitly.
There was a problem hiding this comment.
Thank you for the tip @einfachIrgendwer0815!
I updated the code and pushed the changes, now all the tests pass correctly and I removed all the integration test changes which were not necessary.
I also added a test for this specific case :)
…:domenicomastrangelo/bat into 2935-compatibility-issue-with-cat-piping
…:domenicomastrangelo/bat into 2935-compatibility-issue-with-cat-piping
|
I think I've found a simpler approach. The following should do the trick: diff --git a/src/bin/bat/app.rs b/src/bin/bat/app.rs
index 8f69870f..cf385c88 100644
--- a/src/bin/bat/app.rs
+++ b/src/bin/bat/app.rs
@@ -251,9 +251,10 @@ impl App {
loop_through: !(self.interactive_output
|| self.matches.get_one::<String>("color").map(|s| s.as_str()) == Some("always")
|| self
.matches
.get_one::<String>("decorations")
.map(|s| s.as_str())
== Some("always")
- || self.matches.get_flag("force-colorization")),
+ || self.matches.get_flag("force-colorization")
+ || self.matches.get_flag("number")),
tab_width: selfThat way, whenever |
Updating
print_line function inSimplePrinter to use line numbers when printing a line if line numbers are desired (self.config.style_components.numbers() / StyleComponent::LineNumbers).This was a compatibility issue with cat until now as describe in issue #2935.
Updating of some integration tests was necessary as now we expect the output to contain line numbers.